fix(tooltip): add boundary padding #1065

nickofthyme · 2021-03-09T15:11:20Z

Summary

Adds an artificial padding around the specified boundary element to contain tooltip within boundaries with top margins.

Screen.Recording.2021-03-09.at.09.06.08.AM.mp4

Note: This does not impact the tooltip offset for the achor.

Checklist

Any consumer-facing exports were added to src/index.ts (and stories only import from ../src except for test data & storybook)
Proper documentation or storybook story was added for features that require explanation or tutorials
Unit tests were updated or added to match the most common scenarios

markov00

LGTM, I have tested it locally and works fine.

markov00 · 2021-03-09T17:06:54Z

api/charts.api.md

@@ -2221,6 +2221,8 @@ export interface TooltipInfo {
 // @public
 export interface TooltipPortalSettings<B = never> {
    boundary?: HTMLElement | B;
+    // Warning: (ae-forgotten-export) The symbol "Padding" needs to be exported by the entry point index.d.ts


Do you want to expose also the Padding type?

Good idea 8d9f3f6

markov00 · 2021-03-09T17:09:13Z

src/components/portal/tooltip_portal.tsx

@@ -138,7 +154,7 @@ const TooltipPortalComponent = ({
            boundary,
            // checks main axis overflow before trying to flip
            altAxis: false,
-            padding: offset || 10,
+            padding: addToPadding(boundaryPadding, offset || 10),


with that offset || 10 are we saying that we always want a padding of 10 and the user cannot set 0 here?

I'm not sure why I had 10 there. Maybe just to flip away 10px from the edge.

Anyways I removed this and only add the offset value. See 8d9f3f6

codecov-io · 2021-03-09T19:05:45Z

Codecov Report

Merging #1065 (09c4a91) into master (27be924) will increase coverage by 0.39%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master    #1065      +/-   ##
==========================================
+ Coverage   72.47%   72.86%   +0.39%     
==========================================
  Files         366      382      +16     
  Lines       11361    11672     +311     
  Branches     2472     2523      +51     
==========================================
+ Hits         8234     8505     +271     
- Misses       3112     3144      +32     
- Partials       15       23       +8

Flag	Coverage Δ
unittests	`72.45% <50.00%> (-0.02%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/components/portal/types.ts	`100.00% <ø> (ø)`
src/components/portal/tooltip_portal.tsx	`89.61% <50.00%> (-3.55%)`	⬇️
src/mocks/series/utils.ts	`100.00% <0.00%> (ø)`
src/mocks/geometries.ts	`92.85% <0.00%> (ø)`
src/mocks/specs/index.ts	`100.00% <0.00%> (ø)`
src/mocks/store/index.ts	`100.00% <0.00%> (ø)`
src/mocks/annotations/annotations.ts	`66.66% <0.00%> (ø)`
src/mocks/scale/index.ts	`100.00% <0.00%> (ø)`
src/mocks/series/series_identifiers.ts	`100.00% <0.00%> (ø)`
src/mocks/series/index.ts	`100.00% <0.00%> (ø)`
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27be924...09c4a91. Read the comment docs.

monfera

LGTM!

# Conflicts: # api/charts.api.md # src/index.ts

## [25.0.3](v25.0.2...v25.0.3) (2021-03-09) ### Bug Fixes * **tooltip:** add boundary padding ([#1065](#1065)) ([#1068](#1068)) ([9a44036](9a44036))

# [25.2.0](v25.1.1...v25.2.0) (2021-03-09) ### Bug Fixes * **tooltip:** add boundary padding ([#1065](#1065)) ([25a247e](25a247e)) ### Features * **partition:** flame and icicle performance and tweening ([#1041](#1041)) ([a9648a4](a9648a4))

nickofthyme · 2021-03-09T22:15:39Z

🎉 This PR is included in version 25.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

monfera · 2021-03-10T10:07:25Z

src/components/portal/tooltip_portal.tsx

@@ -56,6 +57,20 @@ type PortalTooltipProps = {
  chartId: string;
 };

+function addToPadding(padding?: Partial<Padding> | number, extra: number = 0): Padding | number | undefined {
+  if (!padding) return undefined;


What if padding is 0, is it OK to return undefined?

Good point, I don't think this is critical but it should return just the extra value when/if padding is undefined. I'll open a pr.

@monfera fixed here #1088

# [25.2.0](elastic/elastic-charts@v25.1.1...v25.2.0) (2021-03-09) ### Bug Fixes * **tooltip:** add boundary padding ([opensearch-project#1065](elastic/elastic-charts#1065)) ([5606eba](elastic/elastic-charts@5606eba)) ### Features * **partition:** flame and icicle performance and tweening ([opensearch-project#1041](elastic/elastic-charts#1041)) ([2ac7550](elastic/elastic-charts@2ac7550))

feat(tooltip): boundary padding

88f2262

nickofthyme added enhancement New feature or request kibana cross issue Has a Kibana issue counterpart :tooltip Related to hover tooltip labels Mar 9, 2021

chore: update api docs

e7b44ad

nickofthyme requested a review from markov00 March 9, 2021 15:15

markov00 approved these changes Mar 9, 2021

View reviewed changes

nickofthyme changed the title ~~feat(tooltip): boundary padding~~ fix(tooltip): add boundary padding Mar 9, 2021

nickofthyme added 2 commits March 9, 2021 13:25

fix: pr comments

8d9f3f6

chore: remove console.log

09c4a91

monfera approved these changes Mar 9, 2021

View reviewed changes

nickofthyme merged commit 25a247e into elastic:master Mar 9, 2021

nickofthyme deleted the tooltip-boundary-padding branch March 9, 2021 20:37

nickofthyme added a commit to nickofthyme/elastic-charts that referenced this pull request Mar 9, 2021

fix(tooltip): add boundary padding (elastic#1065)

81fd257

# Conflicts: # api/charts.api.md # src/index.ts

nickofthyme mentioned this pull request Mar 9, 2021

fix(tooltip): add boundary padding (#1065) [25.0.x] #1068

Merged

nickofthyme added a commit that referenced this pull request Mar 9, 2021

fix(tooltip): add boundary padding (#1065) (#1068)

9a44036

github-actions bot pushed a commit that referenced this pull request Mar 9, 2021

chore(release): 25.0.3 [skip ci]

3236301

## [25.0.3](v25.0.2...v25.0.3) (2021-03-09) ### Bug Fixes * **tooltip:** add boundary padding ([#1065](#1065)) ([#1068](#1068)) ([9a44036](9a44036))

nickofthyme added the released Issue released publicly label Mar 9, 2021

monfera reviewed Mar 10, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(tooltip): add boundary padding #1065

fix(tooltip): add boundary padding #1065

nickofthyme commented Mar 9, 2021 •

edited

Loading

markov00 left a comment •

edited

Loading

markov00 Mar 9, 2021

nickofthyme Mar 9, 2021

markov00 Mar 9, 2021

nickofthyme Mar 9, 2021

codecov-io commented Mar 9, 2021 •

edited

Loading

monfera left a comment

nickofthyme commented Mar 9, 2021

monfera Mar 10, 2021

nickofthyme Mar 16, 2021

nickofthyme Mar 24, 2021

fix(tooltip): add boundary padding #1065

fix(tooltip): add boundary padding #1065

Conversation

nickofthyme commented Mar 9, 2021 • edited Loading

Summary

Checklist

markov00 left a comment • edited Loading

Choose a reason for hiding this comment

markov00 Mar 9, 2021

Choose a reason for hiding this comment

nickofthyme Mar 9, 2021

Choose a reason for hiding this comment

markov00 Mar 9, 2021

Choose a reason for hiding this comment

nickofthyme Mar 9, 2021

Choose a reason for hiding this comment

codecov-io commented Mar 9, 2021 • edited Loading

Codecov Report

monfera left a comment

Choose a reason for hiding this comment

nickofthyme commented Mar 9, 2021

monfera Mar 10, 2021

Choose a reason for hiding this comment

nickofthyme Mar 16, 2021

Choose a reason for hiding this comment

nickofthyme Mar 24, 2021

Choose a reason for hiding this comment

nickofthyme commented Mar 9, 2021 •

edited

Loading

markov00 left a comment •

edited

Loading

codecov-io commented Mar 9, 2021 •

edited

Loading